Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kubeadm/dual stack support in 1.21 #26675

Merged
merged 1 commit into from
Mar 26, 2021

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Feb 23, 2021

part of kubernetes/kubeadm#1612

  • prepare: sudo sysctl -w net.ipv6.conf.all.forwarding=1
  • init args for dual stack
  • how to use single stack
  • --apiserver-advertise-address doesn't support dual-stack yet.
  • kubelet --node-ip supports multi IP now. (how kubeadm use that?) I need test on it.
  • join a dual stack node
  • upgrade behavior changes: ?
  • HA configurations (not in this pr)

https://kubernetes.io/docs/concepts/services-networking/dual-stack/

kubeadm dual-stack install log: https://gist.github.com/pacoxu/3fda7ccbccca82a0cd791cad454ae69f

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language labels Feb 23, 2021
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 23, 2021
@netlify
Copy link

netlify bot commented Feb 23, 2021

Deploy preview for kubernetes-io-master-staging ready!

Built with commit acc69af

https://deploy-preview-26675--kubernetes-io-master-staging.netlify.app

@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from 2973d6d to a6c63a8 Compare February 24, 2021 07:52
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 24, 2021
@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from a6c63a8 to 6e3798c Compare February 24, 2021 10:32
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 24, 2021
@pacoxu pacoxu changed the title [PlaceHolder][WIP]Kubeadm/dual stack support in 1.21 Kubeadm/dual stack support in 1.21 Feb 24, 2021
@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from 6e3798c to 774b910 Compare February 24, 2021 10:34
@pacoxu
Copy link
Member Author

pacoxu commented Feb 24, 2021

apiVersion: kubeadm.k8s.io/v1beta2
featureGates:
  IPv6DualStack: true
kind: ClusterConfiguration
networking:
      podSubnet: 172.30.0.0/16,fefe:ffff:0::/48
      serviceSubnet: 172.31.0.0/16,fefe:ffff:1::/108

Need I add a sample configuration file like?

@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch 2 times, most recently from 823be4f to acd337e Compare February 24, 2021 10:58
@neolit123
Copy link
Member

neolit123 commented Feb 24, 2021

/cc
@kubernetes/sig-cluster-lifecycle

i think including an example config is fine, v1beta2 is not deprecated and will be around for at least one more year (if not more).

EDIT:
we should have api version and kind next to each other:

apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterConfiguration
featureGates:
  IPv6DualStack: true
networking:
      podSubnet: 172.30.0.0/16,fefe:ffff:0::/48
      serviceSubnet: 172.31.0.0/16,fefe:ffff:1::/108

@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from 73a4089 to 1834a72 Compare February 25, 2021 02:11
@pacoxu
Copy link
Member Author

pacoxu commented Feb 25, 2021

Updated the PR

  • ipv6 -> IPv6
  • add an example config yaml

Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine to me, not sure if this should be the page where we add it.

most importantly the PR should be against k/website's dev-1.21 branch.
the "change branch" option of github usually fails for k/website.

you can try:

  • change the PR branch here
  • do git diff ... > patch.diff locally to store the diff
  • delete local branch
  • create a branch with same name based on dev-1.21 locally
  • apply the patch
  • push to the remote

@neolit123
Copy link
Member

@ sig-docs / @ sig -network do you think that we should have this guide in the kubeadm docs and just cross link to it from the sig network dual-stack page?

@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from d164a34 to e5b3758 Compare February 25, 2021 07:12
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 25, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 2, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2021
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 1e5305e96882ef34642bb7d10f120933b0089394

@aojea
Copy link
Member

aojea commented Mar 2, 2021

LGTM

@pacoxu
Copy link
Member Author

pacoxu commented Mar 2, 2021

/assign @jimangel

@pacoxu
Copy link
Member Author

pacoxu commented Mar 6, 2021

/cc @sftim
for approval

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for proposing this new page.

  • I'd prefer not to recommend using fd00::/8 as this range is already deprecated. Let's use an IPv6 documentation IPv6 range instead. I added an example inline.
  • If it's feasible, lets use port 443 as the HTTPS port. This matches typical port assignments on the wider internet.
  • I'm not sure why this page recommends setting fail-swap-on to false.

content/en/docs/concepts/services-networking/dual-stack.md Outdated Show resolved Hide resolved
kind: InitConfiguration
localAPIEndpoint:
advertiseAddress: "10.100.0.1"
bindPort: 6443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In documentation, I recommend using the https port:

Suggested change
bindPort: 6443
bindPort: 443

Port 443 is more recognisable to readers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't object to this, but for kubeadm users are more used to 6443, because that is the default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I will keep it as 6443 or remove it ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my vote is to leave it 6443.

kubeadm init --feature-gates IPv6DualStack=false
```

To make things more clear, here is an example kubeadm [configuration file](https://pkg.go.dev/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2) `kubeadm-config.yml` for the single-stack control plane node.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on how thorough we want to be, it might be useful to point out that single-stack IPv6 is also supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, emphasizing that is a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling the dual-stack feature doesn't mean that you need to use dual-stack addressing.  
You can deploy a single-stack cluster that has the dual-stack networking feature enabled.

Current description. Need I add more?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, seems good.

@@ -0,0 +1,144 @@
---
title: Dual-stack support with kubeadm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The improvement is dual stack support, but once that lands it's just “dual stack” (think about the page about RBAC; once it was new, but the page about it doesn't talk about RBAC support, just about RBAC).

How about:

Suggested change
title: Dual-stack support with kubeadm
title: Enable dual-stack networking using kubeadm

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neolit123 Do you agree to change the title?

Dual-stack support with kubeadm is simple.
I prefer Enable dual-stack networking using kubeadm as Tim says. The page mainly tells about how to enable it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't object.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 8, 2021
@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from de85ed8 to 08064f5 Compare March 8, 2021 03:21
@pacoxu
Copy link
Member Author

pacoxu commented Mar 8, 2021

Thanks for proposing this new page.

  • I'd prefer not to recommend using fd00::/8 as this range is already deprecated. Let's use an IPv6 documentation IPv6 range instead. I added an example inline.
  • If it's feasible, lets use port 443 as the HTTPS port. This matches typical port assignments on the wider internet.
  • I'm not sure why this page recommends setting fail-swap-on to false.

@sftim Thanks for your reveiw.
Updated and rebased, except the HTTPS port as Lubomir commented that it is a default value from kubeadm.

@neolit123
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 22, 2021
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: ba9c2264fad1a177a0325665019ad454a2d4cde2

@reylejano
Copy link
Member

Hi @pacoxu , this PR has a tech lgtm and updates addressing comments from a sig docs reviewer.
@kubernetes/sig-docs-pr-reviews Can you review the updates and give an lgtm?

Also wanted to share upcoming doc related dates for the 1.21 release:

  • Docs Ready for Review deadline is March 24 done
  • Docs Ready to Merge deadline is March 31

@reylejano
Copy link
Member

@pacoxu Can you squash your commits

Thank you

Signed-off-by: pacoxu <paco.xu@daocloud.io>
Co-authored-by: Lubomir I. Ivanov <neolit123@gmail.com>
Co-authored-by: Tim Bannister <tim@scalefactory.com>
@pacoxu pacoxu force-pushed the kubeadm/dual-stack branch from 945010f to 5a6c424 Compare March 26, 2021 02:27
@pacoxu
Copy link
Member Author

pacoxu commented Mar 26, 2021

@reylejano updated.

@tengqm
Copy link
Contributor

tengqm commented Mar 26, 2021

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2021
@k8s-ci-robot k8s-ci-robot merged commit c9916c8 into kubernetes:dev-1.21 Mar 26, 2021
@pacoxu pacoxu deleted the kubeadm/dual-stack branch June 26, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants